fix(metrics): use raw metrics vec in connection metrics #14017
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #14017 +/- ##
==========================================
- Coverage 68.09% 68.08% -0.02%
==========================================
Files 1543 1543
Lines 267096 267096
==========================================
- Hits 181890 181852 -38
- Misses 85206 85244 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Wonder if my understanding is correct... Either of the following should be good, right?
And this PR chose 1. Is this correct? |
Yes, we chose 1, and before this PR, we already use the dimension The drawback of 2 is because of the drawback of the current label guarded metrics. For example, when we measure the connection to S3, which has a fixed endpoint, if we use the label guarded metrics, after a query has finished, the connection is dropped, and metrics of the labels will be dropped and reset and may cause the same incorrect metric as #13948, though we will report metrics of the same label very soon when we make the next S3 query. |
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
#13981
Previously we use
LabelGuardedXxxmetrics vec to report connection metrics. We use the label guarded one to prevent label leak. However, the lifetime of the label guarded metrics is within the lifetime of each connection, and for some short connection such as connection with s3, it is used for short time, and is dropped soon, which will cause the label to be dropped and later reset and the metrics will become inaccurate.However, the label of connection metrics is
[connection_type, endpoint]. The set ofconnection_typeis bounded, andendpointis actually ip, and the set ofendpointis bounded. So for connection metrics, we don't need to use the label guarded metrics. So in this PR, we change to use the raw metrics vec back for connection metrics.Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.